-
Notifications
You must be signed in to change notification settings - Fork 422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor the calculation to avoid reduntant time conversions #2545
Conversation
7504.1 / Erlang 22.0 / small_tests / 64fa7e4 7504.2 / Erlang 22.0 / internal_mnesia / 64fa7e4 7504.3 / Erlang 22.0 / odbc_mssql_mnesia / 64fa7e4 7504.4 / Erlang 22.0 / mysql_redis / 64fa7e4 7504.7 / Erlang 22.0 / elasticsearch_and_cassandra_mnesia / 64fa7e4 7504.6 / Erlang 22.0 / ldap_mnesia / 64fa7e4 7504.5 / Erlang 22.0 / riak_mnesia / 64fa7e4 7504.9 / Erlang 21.3 / pgsql_mnesia / 64fa7e4 |
Codecov Report
@@ Coverage Diff @@
## master #2545 +/- ##
==========================================
+ Coverage 78.99% 79.01% +0.01%
==========================================
Files 343 343
Lines 29494 29491 -3
==========================================
+ Hits 23298 23301 +3
+ Misses 6196 6190 -6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you squash these 2 commits into one?
98f9d00
to
87bde93
Compare
Done :) |
7510.1 / Erlang 22.0 / small_tests / a90f120 7510.2 / Erlang 22.0 / internal_mnesia / a90f120 7510.3 / Erlang 22.0 / odbc_mssql_mnesia / a90f120 7510.4 / Erlang 22.0 / mysql_redis / a90f120 7510.7 / Erlang 22.0 / elasticsearch_and_cassandra_mnesia / a90f120 7510.5 / Erlang 22.0 / riak_mnesia / a90f120 7510.6 / Erlang 22.0 / ldap_mnesia / a90f120 7510.9 / Erlang 21.3 / pgsql_mnesia / a90f120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks fine but we need a separate PR first that would introduce tests that focus on shapers verification.
87bde93
to
35850d5
Compare
7885.1 / Erlang 22.0 / small_tests / 4e51e93 7885.2 / Erlang 22.0 / internal_mnesia / 4e51e93 7885.4 / Erlang 22.0 / mysql_redis / 4e51e93 7885.3 / Erlang 22.0 / odbc_mssql_mnesia / 4e51e93 7885.5 / Erlang 22.0 / riak_mnesia / 4e51e93 7885.7 / Erlang 22.0 / elasticsearch_and_cassandra_mnesia / 4e51e93 7885.6 / Erlang 22.0 / ldap_mnesia / 4e51e93 7885.9 / Erlang 21.3 / pgsql_mnesia / 4e51e93 |
On
shaper.erl
, the arithmetics are done by converting to and fromnative
-second
time units, which shows on the profiling statistics I gathered last time:We see that
convert_time_unit/3
is called twice as often asmonotonic_time/0
, which is exactly whatshaper:update/2
does.On
fprof
, we can see that, of the accumulated 130us thatshaper:update/2
takes, 20us of them are spent onerlang:convert_time_unit/3
, plus 4us onerlang:monotonic_time/0
, whileMy main idea to optimise this, is this quote from the
erlang:monotonic_time/1
docs: